Indexed vector sources #1377
Indexed vector sources #1377
Conversation
|
|
||
| module.exports = TilePyramid; | ||
|
|
||
| function TilePyramid(options) { |
There was a problem hiding this comment.
This and the rest of the new functionality needs JSDoc documentation information to match the rest of mapbox-gl-js.
There was a problem hiding this comment.
Could this be re-titled IndexedTilePyramid to match its difference in functionality?
There was a problem hiding this comment.
I've removed the concept of "indexedVector" for now, and all new files (esri-*) and datasources that were originally in this branch/PR are gone. In its place I've modified 5 files (in each very subtly). Will call each change out separately below.
|
@tmcw Thanks for starting this. The intention of checking the original code in (all the esri-* files) was to have a temp reference to how the original code was changed. I'll remove it since what I've done here is actually remove the concept of an "indexedVector" and have changed the behavior of the existing "vector" data sources. I'll explain the changes better with line notes, and will sync the PR to remove unneeded cruft. |
| @@ -32,7 +33,22 @@ exports._loadTileJSON = function(options) { | |||
| redoPlacement: this._redoTilePlacement ? this._redoTilePlacement.bind(this) : undefined | |||
| }); | |||
|
|
|||
| this.fire('load'); | |||
| // if index is defined, fetch the index json, then extend the pyramid | |||
| if (tileJSON.index) { | |||
There was a problem hiding this comment.
This is the entry point for the difference in behavior. If an index exists (as a url) on the data source "tileJSON" the index gets requested. The returned index is added to the tile pyramid.
|
Just to comment on the status here: The team inside Esri is focusing on cooking up some benchmarks on using a tile index across our stack. This will help us understand the impact of the index on tile generation (less tiles) and the impact at render time (data clipping vs requesting/rendering very small tiles). It may be some time still, but everyone is eager to keep this moving forward. |
… relationship to its parent
…or the correct parent tile
ycabon
force-pushed
the
indexed-vector-sources
branch
from
56bb519 to
8140de4
Compare
|
Here is a gist that discusses the concept of the Indexing of Vector Tiles and the justification for Clipping. https://gist.github.com/odoe/ce6a150658526901ef27#file-vector-tile-pr-md |
|
related PR mapbox/vector-tile-js#37 |
|
Tests used to compare processing times for indexed tile vs. non-indexed tile - https://github.com/jcardonadcdev/vt-performance-tests |
|
The Esri side of this fork hasn't been updated since November 2015, and Mapbox GL JS has changed massively since then. It doesn't look like there are any active branches on the Esri project anymore. Happy to take a look at an updated branch that resolves some of the initial issues outlined in this PR! Closing this specific PR as stale. |
|
😞 |
Style, context, polish
demo/mapbox-gl-dev.jsanddemo/streets-mobile.jsonprobably shouldn't be checked inesri-files follow a dashed naming convention that diverges from the underscored naming convention of the rest of the libesri-code needs JSDoc strings like the rest of the libesri-tile-pyramiddiffers more in functionality than proprietariness to theTilePyramidcode, and it is exposed via GL styles asindexedVector: should it be titledIndexedTilePyramidinstead?indexedVector tech
merge blockers
High-level there are three outcomes:
/cc @chelm @jcardonadcdev @ycabon